Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Ready] Docs changes to remove pandas-iris and update kedro new flow in onboarding docs #3317

Merged
merged 45 commits into from
Nov 27, 2023
Merged

[Ready] Docs changes to remove pandas-iris and update kedro new flow in onboarding docs #3317

merged 45 commits into from
Nov 27, 2023

Conversation

stichbury
Copy link
Contributor

@stichbury stichbury commented Nov 16, 2023 •
edited
Loading

NOTE: Kedro datasets are moving from kedro.extras.datasets to a separate kedro-datasets package in
kedro-plugins repository. Any changes to the dataset implementations
should be done by opening a pull request in that repository.

Description

To address #3048 by updating the new project creation flow.

Ready for first review by @amandakys.

PREVIEW CODE IS BUILT HERE

Development notes

I have updated the new project text but have also:

  • split the starters documentation into two sections for usage and starter creation, both are tidied up for language and content
  • created a new folder for docs about starters and what I've currently titled "new project tools"
  • revised text throughout the docs that refers to pandas-iris

Still to come

Developer Certificate of Origin

We need all contributions to comply with the Developer Certificate of Origin (DCO). All commits must be signed off by including a Signed-off-by line in the commit message. See our wiki for guidance.

If your PR is blocked due to unsigned commits, then you must follow the instructions under "Rebase the branch" on the GitHub Checks page for your PR. This will retroactively add the sign-off to all unsigned commits and allow the DCO check to pass.

Checklist

  • Read the contributing guidelines
  • Signed off each commit with a Developer Certificate of Origin (DCO)
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the RELEASE.md file
  • Added tests to cover my changes
  • Checked if this change will affect Kedro-Viz, and if so, communicated that with the Viz team

@stichbury
Revise link to notebook docs and remove unnecessary intro page
9e9449c 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Update starters content
c5b5b0d 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'develop' into fix-starters-content
ce80d99
@stichbury
relocate starters content
3cee9f4 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Added some changes for add-ons and some to do notes
124bd90 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'develop' into fix-starters-content
b58d9a4
@stichbury
Copy link
Contributor Author

stichbury commented Nov 20, 2023 •
edited
Loading

Some notes on what we need to do to complete this PR:

  • Update the get started section to introduce the new project creation flow that uses the different options / remove Iris starter etc. @stichbury to do this
  • Add a page devoted to the different options for the new project creation flow. I've called it "Addons" right now but that needs to be changed (page name, titles etc). We seem to have settled on "Tools" although I'm not massively confident in how to use the terminology so hope @AhdraMeraliQB you can write the page and see how many adjustments are needed. It's coming in this PR Add tools documentation page #3331
  • Revision to the page about Kedro starters (for usage) @AhdraMeraliQB can you take this too please?
  • Introduction of a new page (in the "Extend Kedro" section) which takes the old content about how to create a starter and updates it as needed. @stichbury to do the first cut of this.
  • Review the rest of the docs where kedro new is discussed and adjust (particularly to remove the Iris starter and add tools options). See Revise documentation that uses the iris starter #3160 for a list, which I've worked through except for one query about generators (requested input from @noklam and @merelcht)

@stichbury stichbury mentioned this pull request Nov 21, 2023
Closed
13 tasks
stichbury and others added 10 commits November 21, 2023 15:23
@stichbury
Remove mention of pandas-iris where possible, replacing with alternative
cd7938e 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'develop' into fix-starters-content
8724441
@stichbury
Fix linter errors
c4d8ed5 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'fix-starters-content' of https://github.com/kedro-org/k…
f7cbffa 
…edro into fix-starters-content
@stichbury
Update new project docs
fc4aed5 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@AhdraMeraliQB
Merge branch 'develop' into fix-starters-content
2349b7d
@stichbury
Remove deprecated starters from architecture diagram
bdf10a3 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'fix-starters-content' of https://github.com/kedro-org/k…
6e678e6 
…edro into fix-starters-content
@stichbury
Add warning for pandas-iris usage in generator section
f2a8868 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Further updates for instances of kedro new
50b51c9 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
AhdraMeraliQB
AhdraMeraliQB reviewed Nov 22, 2023
View reviewed changes
docs/source/get_started/new_project.md Outdated
```bash
kedro new --config=<path>/config.yml
```
The configuration file must contain:

- `output_dir` The path in which to create the project directory
- `project_name`
- `repo_name`
- `python_package`
Copy link
Contributor

@AhdraMeraliQB AhdraMeraliQB Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add-ons are also configurable here - will double check what happens when they are omitted

Update: The process fails if they are omitted, consistent with the other attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll need some insight on how to include these so will await the changes on your branch for guidance.

Ahdra Merali added 2 commits November 22, 2023 12:05
Remove TODO as no longer required
5db4cad 
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Merge
b397ccf 
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
@stichbury
fix cookiecutter docs urls
135ad6a 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Update the create a starter docs
711bc0b 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'fix-starters-content' of https://github.com/kedro-org/k…
ccc8f96 
…edro into fix-starters-content
astrojuanlu
astrojuanlu reviewed Nov 23, 2023
View reviewed changes
docs/source/faq/faq.md Outdated Show resolved Hide resolved
@astrojuanlu
Copy link
Member

I gave a look and looks okay! I see there are some parts yet to be written still

stichbury and others added 5 commits November 23, 2023 13:57
@stichbury
Fix link to avoid linkcheck barf
201776d 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury @astrojuanlu
Update docs/source/get_started/new_project.md
b02d672 
Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Update following review
57701f6 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Update content
aec7ebc 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'fix-starters-content' of https://github.com/kedro-org/k…
59fdfcf 
…edro into fix-starters-content
@stichbury
Copy link
Contributor Author

This is ready for final review @astrojuanlu and @merelcht @amandakys

The sections marked as "to do" will be covered by @AhdraMeraliQB and @SajidAlamQB in a separate PR (branched from this branch) here #3331

merelcht
merelcht approved these changes Nov 24, 2023
View reviewed changes
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I've left some minor comments.

docs/source/nodes_and_pipelines/nodes.md Outdated Show resolved Hide resolved
docs/source/starters/create_a_starter.md Outdated Show resolved Hide resolved
docs/source/starters/starters.md Outdated Show resolved Hide resolved
docs/source/starters/starters.md Show resolved Hide resolved
astrojuanlu
astrojuanlu reviewed Nov 24, 2023
View reviewed changes
Copy link
Member

@astrojuanlu astrojuanlu left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gave this a more thorough review. The content is fantastic - I have a few reservations about the overall structure of the starters folder. I made some specific suggestions in the comments and opened some questions.

docs/source/starters/index.md Outdated
@@ -0,0 +1,44 @@
# New project tools
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that /starters/ is for "new project tools", /starters/starters.html is for "starters", and /starters/new_project_tools.html is for "configure a new project" (looking at the H1 of each page).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And speaking specifically about this title, "new project tools" could be read as "tools for new projects" or "new thing called project tools", which was confusing to me at first. I bet we don't want to use the second meaning, given that the "new" tools will stop being new at some point...

Copy link
Contributor Author

@stichbury stichbury Nov 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should fix the naming in the upcoming PR from @AhdraMeraliQB which explicitly writes the content. I can't really guess at what to call the page right now, so that's a placeholder.

You're correct about the /starters being an odd place for those tools but it's awkward to name the folder. Maybe "/project_creation. Again, I'm pushing that to #3331 rather than resolving here.

docs/source/starters/index.md Outdated Show resolved Hide resolved
docs/source/index.rst Show resolved Hide resolved
docs/source/extend_kedro/index.md Show resolved Hide resolved
docs/source/starters/index.md Show resolved Hide resolved
docs/source/starters/starters.md Show resolved Hide resolved
docs/source/starters/starters.md Show resolved Hide resolved
@amandakys
Copy link

I looked at the New Project Tools page and the Create a new Kedro project pages.

I like the way the two pages link together, the Create a New Kedro Project page is now much clearer and flows well.

On the New Project Tools page, would it be worth highlighting the kedro new --addons flag since we are listing out flags to use with the kedro new command? Or are we documenting that elsewhere?

On the Create a New Kedro Project. Would it worth having a section called Project Name? That way the in page navigation will reflect the structure of the CLI. Otherwise, we could signpost that there are 3 steps to the kedro new command at the top and list the steps before we go into more detail?

stichbury and others added 5 commits November 27, 2023 10:23
@stichbury @merelcht
Update docs/source/nodes_and_pipelines/nodes.md
8c38a11 
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury @merelcht
Update docs/source/starters/starters.md
bd04596 
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Update FAQ
735ed4f 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury
Merge branch 'develop' into fix-starters-content
c09cac5
@stichbury
Updates following review
7cb1dd4 
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
@stichbury stichbury enabled auto-merge (squash) November 27, 2023 13:11
@stichbury stichbury merged commit 01e6929 into develop Nov 27, 2023
11 checks passed
@stichbury stichbury deleted the fix-starters-content branch November 27, 2023 13:40
stichbury added a commit that referenced this pull request Nov 28, 2023
@stichbury @AhdraMeraliQB
@astrojuanlu @merelcht
[Ready] Docs changes to remove pandas-iris and update kedro new f…
33aa83d 
…low in onboarding docs (#3317)

* Revise link to notebook docs and remove unnecessary intro page

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update starters content

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* relocate starters content

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Added some changes for add-ons and some to do notes

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Some further fixes

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Move section about development version of Kedro

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Add text for new project

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Remove mention of pandas-iris where possible, replacing with alternative

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Fix linter errors

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update new project docs

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Remove deprecated starters from architecture diagram

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Add warning for pandas-iris usage in generator section

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Further updates for instances of kedro new

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Remove TODO as no longer required

Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>

* Resolve some Vale issues and remove implication of tools + starters

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* fixes to internal links

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* pandas-spaceflights bad, spaceflights-pandas good

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* fix cookiecutter docs urls

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update the create a starter docs

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Fix link to avoid linkcheck barf

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update docs/source/get_started/new_project.md

Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update following review

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update content

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update docs/source/nodes_and_pipelines/nodes.md

Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update docs/source/starters/starters.md

Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Update FAQ

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

* Updates following review

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>

---------

Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Co-authored-by: Ahdra Merali <90615669+AhdraMeraliQB@users.noreply.github.com>
Co-authored-by: Ahdra Merali <ahdra.merali@quantumblack.com>
Co-authored-by: Juan Luis Cano Rodríguez <juan_luis_cano@mckinsey.com>
Co-authored-by: Merel Theisen <49397448+merelcht@users.noreply.github.com>
Signed-off-by: Jo Stichbury <jo_stichbury@mckinsey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@merelcht merelcht merelcht approved these changes

@noklam noklam noklam left review comments

@astrojuanlu astrojuanlu astrojuanlu approved these changes

@yetudada yetudada Awaiting requested review from yetudada yetudada is a code owner

@amandakys amandakys Awaiting requested review from amandakys

@AhdraMeraliQB AhdraMeraliQB Awaiting requested review from AhdraMeraliQB

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stichbury @astrojuanlu @amandakys @noklam @merelcht @AhdraMeraliQB